-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #1251 Add explicit defer
parameter to VirtualTimeScheduler.advanceTime_
methods
#2012
Conversation
…dvanceTime_` methods
@yarosla Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
@yarosla Thank you for signing the Contributor License Agreement! |
defer
parameter to VirtualTimeScheduler.advanceTime_
methodsdefer
parameter to VirtualTimeScheduler.advanceTime_
methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I don't think this approach works, because it puts the empty check outside the drain()
loop, which can be reached by other path than advanceTime
. there can also be a situation where 2 parallel calls to advanceTime are made, with different options (although that should be way more rare). All in all, putting this parameter at the method level makes things more brittle and harder to reason about.
Given that, another possible approach would be to create the VirtualTimeScheduler
with a final defer
option. In the drain()
, each !queue.isEmpty()
check would also be checking if defer == false
You also went against my requirement that the defer
mode had a default of true
.
Your requirements were:
I will look into this closer and make changes next weekend. |
@yarosla ah indeed, my bad. yeah please explore the alternative solution of setting this once and for all at VTSheduler creation when you have time 👍 |
Created new pull request #2017 |
By default, VTS created manually don't defer the advancing of time anymore, even if task queue is empty. Those created implicitly by StepVerifier still do however, to maintain the behavior in the case where deferring advanceTime is most likely to be necessary. See-Also: #2012
forgot to close that one, since #2017 has superseded it |
No description provided.